OpenConceptLab/ocl_issues#2984 | A review note on an unmapped Mapper row is saved - #124

One review pass, P1–P5 considered.

Findings:

1. [P2] Explicitly unmapped rows can be emitted twice in matches: MapProject.jsx keeps rows that were manually unmapped, rejected, excluded, or bulk-cleared in mapSelected with a null value. The existing map(mapSelected, ...) therefore already creates a match entry for those rows. unmappedNoteMatches() tests only !mapSelected[index], so a noted row with a null entry is appended again, giving the save payload two entries with the same rowIndex. Please build one entry per row (or distinguish absent keys from present null keys) and add the mapSelected = {0: null} regression case.

2. [P3] The requested autosave/reopen path is not exercised: the new test calls two pure helpers and serializes their output, but never exercises the onSave wiring that appends to matches, nor the reopen wiring in setProjectFromData. That is why the duplicate-row case above passes unnoticed. Please cover the payload assembly and restore path closely enough to represent the issue’s edit → autosave → reopen acceptance criterion.

Checks: full test suite passes (651/651); focused ESLint passes. No P1, P4, or P5 findings. GitHub does not allow a request-changes verdict on one’s own PR, so this is posted as a review comment.

(snyaggarwal • Codex review)

Re-review of e403fac (P1–P5 considered): no findings.

The previous P2 is resolved: buildMatchEntries now distinguishes an absent key from an explicitly cleared null row and emits one entry per row. The regression case covers the cleared-row path. The extracted helper is used directly by onSave, and the round-trip coverage exercises the same builder and restore helper, resolving the previous P3 test gap.

Checks: full suite passes (652/652); focused ESLint passes; diff check passes. GitHub does not permit approving one’s own PR, so this is recorded as a clean review comment.

(snyaggarwal • Codex re-review)